<fix>[ha]: ZSTAC-83890 add pre-fence peer fallback#4135
Open
zstack-robot-1 wants to merge 1 commit into
Open
Conversation
Allow HA pre-fence pending tags to record accessiblePeerHostUuid as none when the checker cannot provide a peer. The late KVM pre-fence extension resolves that marker after host allocation. Resolves: ZSTAC-83890 Change-Id: I36426b3386f1080ddeae8396ea9c4f7a81e25711
总览该PR在 HA 高可用性功能中，添加了一个系统标签常量，并调整了预栅栏待创建标签的创建逻辑，使其在对端主机UUID缺失时可以使用默认值继续创建标签而非跳过。 变更HA 预栅栏标签改进
可能相关的 PR
预估审查工作量🎯 1 (Trivial) | ⏱️ ~5 分钟 诗句
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ast-grep (0.42.3)compute/src/main/java/org/zstack/compute/vm/VmInstanceBase.javaComment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
compute/src/main/java/org/zstack/compute/vm/VmSystemTags.java (1)
324-324: ⚡ Quick win将哨兵常量改为不可变（Line 324）
该值被设计为跨流程约定的标记值，建议声明为
public static final，避免被误改导致预栅栏匹配逻辑异常。建议修改
- public static String HA_PRE_FENCE_ACCESSIBLE_PEER_HOST_UUID_NONE = "none"; + public static final String HA_PRE_FENCE_ACCESSIBLE_PEER_HOST_UUID_NONE = "none";As per coding guidelines “常量命名：全部大写，使用下划线分隔单词”，该命名语义对应常量，建议保持不可变。
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@compute/src/main/java/org/zstack/compute/vm/VmSystemTags.java` at line 324, The field HA_PRE_FENCE_ACCESSIBLE_PEER_HOST_UUID_NONE in class VmSystemTags should be made an immutable constant to prevent accidental modification; change its declaration to be public static final and keep the String value "none" (i.e., make HA_PRE_FENCE_ACCESSIBLE_PEER_HOST_UUID_NONE a public static final String) so the pre-fence matching logic uses an immutable, conventional constant.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@compute/src/main/java/org/zstack/compute/vm/VmSystemTags.java`:
- Line 324: The field HA_PRE_FENCE_ACCESSIBLE_PEER_HOST_UUID_NONE in class
VmSystemTags should be made an immutable constant to prevent accidental
modification; change its declaration to be public static final and keep the
String value "none" (i.e., make HA_PRE_FENCE_ACCESSIBLE_PEER_HOST_UUID_NONE a
public static final String) so the pre-fence matching logic uses an immutable,
conventional constant.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: http://open.zstack.ai:20001/code-reviews/zstack-cloud.yaml (via .coderabbit.yaml)
Review profile: CHILL
Plan: Pro
Run ID: 87eeb2ee-3f82-4846-ab1f-f9ec008b0dbd
📒 Files selected for processing (2)
compute/src/main/java/org/zstack/compute/vm/VmInstanceBase.javacompute/src/main/java/org/zstack/compute/vm/VmSystemTags.java
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Root Cause
Some HA checker paths can trigger HA without an accessiblePeerHostUuid. When this happens, core HA previously skipped creating haPreFencePending, so the KVM pre-fence extension had no chance to kill the old qemu process before starting the VM elsewhere.
Solution
noneas a deferred marker. The premium KVM pre-fence extension resolves it to the selected destination host after scheduling.Testing
git diff --checkRelated
Resolves: ZSTAC-83890
sync from gitlab !10039